Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export dbus address for the notification server #23

Merged
merged 1 commit into from
May 31, 2024

Conversation

ben-grande
Copy link
Contributor

Only screenshare script requires as of today, but put it in webcam also for future proof as it doesn't cause any harm.

Assigning variable and declaration made separate due to ShellCheck warning SC2155.

For: QubesOS/qubes-issues#6426
Fixes: QubesOS/qubes-issues#8457
Fixes: https://github.com/QubesOS/qubes-video-companion/issues/15

Comment on lines 6 to 8
DISPLAY=:0
DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus"
export DISPLAY DBUS_SESSION_BUS_ADDRESS
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DISPLAY=:0
DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus"
export DISPLAY DBUS_SESSION_BUS_ADDRESS
set -eu
DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus"
export DISPLAY=:0 DBUS_SESSION_BUS_ADDRESS

Comment on lines 6 to 8
DISPLAY=:0
DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus"
export DISPLAY DBUS_SESSION_BUS_ADDRESS
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DISPLAY=:0
DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus"
export DISPLAY DBUS_SESSION_BUS_ADDRESS
set -eu
DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus"
export DISPLAY=:0 DBUS_SESSION_BUS_ADDRESS

@DemiMarie
Copy link
Collaborator

Should $XDG_RUNTIME_DIR also be set?

@ben-grande
Copy link
Contributor Author

Should $XDG_RUNTIME_DIR also be set?

Not necessary but nice to have and be the base path of DBUS_SESSION_BUS_ADDRESS. Will do.

Copy link
Member

@marmarek marmarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't override those unconditionally. Set only if not present already.
Especially, if user session is running (and so is qrexec-fork-server), they should be provided already. There may be cases when either of them have different value (like in GUI domain that runs full desktop env)

Only screenshare script requires as of today, but put it in webcam also
for future proof as it doesn't cause any harm.

Assigning variable and declaration made separate due to ShellCheck
warning SC2155.

For: QubesOS/qubes-issues#6426
Fixes: QubesOS/qubes-issues#8457
Fixes: https://github.com/QubesOS/qubes-video-companion/issues/15
@ben-grande
Copy link
Contributor Author

Don't override those unconditionally. Set only if not present already.

Done.

Especially, if user session is running (and so is qrexec-fork-server), they should be provided already. There may be cases when either of them have different value (like in GUI domain that runs full desktop env)

Unfortunately they were not set and many users, including me, encountered that issue. Now it is using the existent value if set, else fallback.

@ben-grande ben-grande mentioned this pull request May 31, 2024
@marmarek marmarek merged commit a7b8122 into QubesOS:main May 31, 2024
1 of 2 checks passed
@ben-grande ben-grande deleted the dom0-dbus branch May 31, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dom0 screenshare does not work Notification issue of qubes-video-companion-dom0
3 participants